db: opt-in ReadAhead of Table with limited "ahead window"#21880
Merged
Conversation
AskAlexSharov
requested review from
mh0lt,
sudeepdino008 and
yperbasis
as code owners
Contributor
There was a problem hiding this comment.
Pull request overview
Adds an opt-in mechanism to warm specific MDBX tables into the OS page cache to improve operation when chaindata is much larger than RAM, and wires this warmup into several read-heavy workflows (collate/prune, backup/copy, integration stages).
Changes:
- Extend
kv.RoDB/kv.TemporalRoDBwithWarmupTable/WarmupAPIs and provide implementations (MDBX + no-ops for remote/memory DBs). - Integrate per-table warmup into state domain/inverted-index collate/prune and into backup table clear/copy paths.
- Update call sites/signatures in stagedsync resets and integration commands to pass the DB handle where needed; adjust
stage_execflow aroundBuildFiles.
Reviewed changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| polygon/db/database.go | Exposes WarmupTable on Polygon DB wrapper by delegating to underlying kv.RoDB. |
| execution/stagedsync/stage_custom_trace.go | Updates backup.ClearTables call to pass the DB handle. |
| execution/stagedsync/rawdbreset/reset_stages.go | Adds DB arg to reset helpers and passes it to backup.ClearTables for warmup-while-clear. |
| db/state/inverted_index.go | Adds kv.RoDB field and fires background WarmupTable before reading keys table in collate. |
| db/state/domain.go | Fires background WarmupTable before reading domain values table in collate/prune. |
| db/state/aggregator.go | Wires Aggregator DB handle into registered Domains/InvertedIndexes (enables warmup). |
| db/rawdb/blockio/block_writer.go | Passes db into backup.ClearTables when truncating bodies-related tables. |
| db/kv/temporal/kv_temporal.go | Adds TemporalRoDB.Warmup(ctx, force) implementation via underlying MDBX tx warmup. |
| db/kv/remotedb/kv_remote.go | Implements no-op Warmup/WarmupTable for remote DB provider. |
| db/kv/membatchwithdb/memory_store.go | Implements no-op WarmupTable for in-memory store DB. |
| db/kv/membatchwithdb/memory_mutation.go | Implements no-op Warmup/WarmupTable for temporal in-memory DB wrapper. |
| db/kv/mdbx/kv_mdbx.go | Implements MDBX WarmupTable via parallel prefix/seek scans. |
| db/kv/kv_interface.go | Extends interfaces with WarmupTable and Warmup API contracts. |
| db/kv/helpers.go | Forwards WarmupTable through gatedRoDB. |
| db/kv/backup/backup.go | Adds warmupWhile helper; warms tables during Kv2kv and ClearTables; updates signatures. |
| common/dbg/experiments.go | Adds env-controlled WARMUP_TABLE_WORKERS knob (default 0 disables warmup). |
| cmd/integration/commands/stages.go | Updates reset call signature; modifies stageExec commit/build flow. |
| cmd/integration/commands/reset_state.go | Updates backup.ClearTables call to pass DB handle. |
| cmd/integration/commands/refetence_db.go | Updates backup.Kv2kv call signature (drops threads arg). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
AskAlexSharov
changed the title
AskAlexSharov
force-pushed
the
alex/warmup_table_36
branch
from
a9c47e9 to
d3aa6e4
Compare
Member
|
A few substantive concerns on the warmup logic before this comes out of WIP: 1. No parallelism on counter-keyed tables. 2. The 1000-goroutine second loop is redundant. 3. Long-lived read txns. Each prefix scan wraps the full iteration in one 4. The Minor: the new |
AskAlexSharov
force-pushed
the
alex/warmup_table_36
branch
from
d3aa6e4 to
6bf1e15
Compare
AskAlexSharov
force-pushed
the
alex/warmup_table_36
branch
3 times, most recently
from
b83f030 to
6aea736
Compare
AskAlexSharov
changed the title
- ReadAhead.run: defer cancel() so the child context and its ctx-cancel watcher goroutine are released even if a caller forgets Close() (the nil-safe API invites fire-and-forget). - DistributeCursors doc: interior boundaries are tx-owned and invalidated by any same-tx write — clone before mutating (matches how DistributeBounds uses them). - ClearTables doc: trim scope/fallback narration, keep the atomicity/deadlock why.
Collaborator
Author
|
@yperbasis cr plz |
yperbasis
previously requested changes
Jul 6, 2026
Member
There was a problem hiding this comment.
Reviewed at head 6e52947. No correctness bugs found. Findings by severity:
Should fix
db/kv/readahead.go:126— the ~1GB ahead-window guarantee breaks on dup-heavy DupSort tables: interior bounds are key-granular and equal keys are deduped, so a table with few distinct keys collapses to ~1 chunk regardless of size (repro: 1-distinct-key 124MB DupSort table → a single chunk holding all 2.2M entries). One hot key with a >1GB dup subtree (e.g. the Transfer topic in LogTopicsIdx with a large unfrozen tail) makeswarm()fault the entire table at once. Key-granular bounds can't split a dup run, so the fix is clamping bytes faulted insidewarm()(stop ~1GB ahead ofSetPos).
Medium (efficiency of the new path)
db/kv/backup/backup.go:220—len(bounds) < 2never fires for non-nil bounds (DistributeCursorsalways returns ≥ 2 entries; for sizes in [32MB, 64MB)n==1returns exactly[nil, nil]), so the one-chunk case takes full-table read-ahead + whole-tableDeleteRangeinstead of the native drop the comment intends. Should be<= 2.db/kv/readahead.go:234—warm()viatx.Rangecosts ~3 cgomdbx_cursor_getper key (redundantCurrentin bothHasNextandNext, plus the advance). A raw cursorSeek/Nextloop bounded bybytes.Compare(k, to)— the same pattern asbackupTable's copy loop — is 1 cgo call per key on a pass that exists only to fault pages.db/kv/mdbx/delete_range_test.go— the chunkedDeleteRangepath has no DupSort coverage, yet ResetExec/StageCustomTraceReset clear >32MB DupSort tables through it when the flag is on. It behaves correctly today (verified out-of-repo: 30k×8 and 50×20k key/dup shapes, exact counts,Count==0), but nothing in-repo pins it — please add a DupSort chunked-clear case.
Low (design/hygiene)
db/kv/readahead.go:91— over-synchronized:consumerChunk's only pure read is already undermu(a plain int64 mutated undermusuffices);bounds/logLvlare set-once and could be plain fields assigned beforego r.run(...)(which also removes theSetPosstartup nil-window); thewarming/idlegauges + dedicated logger goroutine feed one 20s line both callers already log themselves.db/kv/backup/backup.go:194— the newdb kv.RoDBparam (inert unlessWARMUP_TABLE_WORKERS>0) could instead be recovered from the tx via an optionalinterface{ DB() kv.RoDB }onMdbxTx+ temporal forwarder — the same pattern asHasDeleteRangein this PR — undoing the call-site ripples and theResetSendersreshape.db/kv/temporal/kv_temporal.go:396— temporalRwTxsatisfiesHasDeleteRangeunconditionally, soclearTable's!ok → ClearTabledegrade is dead code for temporal txs, and a future non-mdbx inner would hard-error mid-clear while the siblingDistributeCursorsforwarder silently degrades. Forward the capability conditionally, or unify the two fallback policies.db/kv/helpers.go:182—Deprecated: use NewReadAhead.is misleading:NewReadAheadhas no bounded-amount mode, so the two DumpTxs callers can't migrate mechanically. Either migrate them or drop the marker.
Nits
db/kv/backup/backup.go:248— aBucketSizefailure inside the 20s progress tick aborts the whole clear; log-and-continue (the trigger is near-unreachable, but coupling operation success to a progress line is wrong).db/kv/mdbx/delete_range_test.go:34—u64Keyduplicatesu64tobalready in this package.
- readahead: cap warm() at ~1GB/chunk so a dup-heavy DupSort chunk (one key with a multi-GB dup run, which branch-level DistributeCursors can't split) doesn't fault the whole table at once; rewrite warm() as a raw-cursor Seek/Next loop (1 cgo Get/key vs Range's ~3); drop the redundant internal progress logger + gauges (callers already log); make bounds a set-once field and consumerChunk a plain int64 under mu (removes the atomics and the SetPos startup nil-window). - backup.clearTable: one-chunk case (bounds==[nil,nil], len 2) now takes the native drop (was len<2, never true) instead of full-table warm + whole-table DeleteRange; a BucketSize failure in the progress tick no longer aborts the clear. - temporal: DistributeCursors forwarder fails loud on a non-mdbx inner, matching DeleteRange (was silent degrade — inconsistent). - reset_stages: wrap ClearTables errors with stage context. - helpers: drop the misleading `// Deprecated:` marker (NewReadAhead has no amount-bounded mode the DumpTxs callers could migrate to). - tests: DupSort DeleteRange + chunked-clear coverage; DupSort skew test proving DistributeCursors splits at main-key granularity (hot key not split across chunks).
Collaborator
Author
|
@yperbasis thanks — addressed the head-6e529478d round in 8297a0c: Medium
Low
Nits
Deferred — the
|
…able at chunk granularity)
Collaborator
Author
|
About DupSort sub-keys chunks equality: So, if you want you can create follow-up issue about "ReadAhead of DupSort tables, when table >> RAM, when 1 key has 1Bil of sub-keys, after
|
Collaborator
Author
|
How it looks now no 500gb db: |
taratorio
approved these changes
Jul 7, 2026
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
A Copilot autofix dropped the `if !allSet { break }` guard in the
cursor-position loop, turning the expected end-of-prefix Current error on
unset surplus cursors into a hard failure. This broke
TestSplitBucketByCountFewerPositions (fewer positions than n).
Bump mdbx-go to v0.40.3, which maps MDBX_ENODATA (GET_CURRENT on a
never-positioned cursor) to the new ErrNoData sentinel. Current() now
treats it as end-of-data (k==nil), like NOTFOUND.
DistributeCursors leaves surplus cursors hollow when the range has fewer
positions than n; Current() on them used to leak ENODATA as a hard error,
worked around by a blanket `if !allSet { break }` that also swallowed real
faults. With ErrNoData normalized, drop that band-aid: the existing
k==nil check ends the prefix and genuine errors propagate.
4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Completely changed PR.
Now:
mdbx_cursor_distribute()1gbahead)ClearTable()method now based onmdbxgo_cursor_delete_range()+ReadAheader()It greatly speedup "clear table" and "prune" operations on
chaindata >> RAMcaseSpeedup tools:
integration stage_exec --resetintegration mdbx_to_mdbxReadAhead is behind
WARMUP_TABLE_WORKERS=256feature-flag.By default not enabled. But maybe i will auto-enable it - because it's used on offline-maintainance-tools only.
FYI:
mdbx_cursor_distribute()is fast ondb >> rambecause i run it only on BranchNodes of btree:table.stats.depth - 2